-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[KEYCLOAK-11746] More unit tests: browser flow #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is good, but I think that with the refactoring of the manner in which flows are built for the tests, we can create a class in org.keycloak.testsuite.util that will be of more general use.
} | ||
|
||
private static void addAuthenticatorExecutionSubFlow(RealmModel realm, Requirement requirement, int priority, AuthenticationFlowModel subFlow, AuthenticationFlowModel parentFlow) { | ||
AuthenticationExecutionModel execution = new AuthenticationExecutionModel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do a bit better here: have the method create the flow and the executor.
} | ||
|
||
private static void addAuthenticatorExecution(RealmModel realm, Requirement requirement, String providerId, int priority, AuthenticationFlowModel parentFlow) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods are really useful, and I think that we would benefit from moving them to a util class rather than just keeping them here. For example, they could also be used to simplify the code in KcOidcFirstBrokerLoginNewAuthTest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The activate flow shouldn't be restricted to browser, but otherwise it's OK.
return this; | ||
} | ||
|
||
public void activateFlow() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be used for more than just browser flows. So either the tool should know which flow to replace from a previous step, or it should be defined here.
cf6a6c4
to
50060a9
Compare
50060a9
to
b2fde54
Compare
Replaced by #19 |
…loak#17380) Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com> (cherry picked from commit ec81091)
…session (#18) (keycloak#17380) (cherry picked from commit e2b5cb4)
* KEYCLOAK-15527 Adding context to master.adoc and index.adoc to allow reuse * KEYCLOAK-15614 modularisation of 'Creating an Open ID Connect Client' (cloudtrust#14) * More changes based on feedback * Puts back 2 out of 3 screens * KEYCLOAK-15614 updates to anchors * Minor spelling correction in Getting Started * Keycloak 15786 (cloudtrust#16) * rebase corrections * post feedback changes * post review changes (cloudtrust#18) * KEYCLOAK-15616 initial commit Co-authored-by: Andy Munro <amunro@redhat.com>
…loak#17380) (keycloak#17389) Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>
No description provided.